Do not fail the sweep when a head branch is reused by a newer pull request - #9333
Merged
Conversation
…quest Now that the sweep runs to completion it surfaces a condition that was previously unreachable. When a contributor closes a pull request and then reuses its head branch for a new one, the head branch no longer points at the SHA the closed pull request recorded. `delete_branch` correctly refuses to delete it -- deleting it would break the newer pull request -- but it reported that refusal as a failure, so the whole run went red. That state is permanent for as long as the newer pull request stays open, so the workflow would have been red on every run indefinitely. That is the same alert fatigue the previous fix set out to remove, just from a different cause. `jdeniau-GHSA-xvcm-6775-5m9r` is a live example: pull request 9129 was closed, one more commit was pushed to its head branch, and pull request 9226 was opened from it and is still open. Reusing a branch this way is a normal contributor pattern here, not an operational fault, and it needs no human intervention: once the newer pull request is closed its own cleanup removes the branch, the older pull request's head then reads as already absent, and the leftover staging branch is collected on the next sweep. Treat it the way the other "nothing safe to do here" states are already treated -- still open, fork-headed, deleted -- and skip it instead of failing. `delete_branch` now returns 3 for this case specifically, so a genuine deletion failure is still a failure and still stops the staging branch from being deleted after it. Nothing that was previously deleted is deleted any differently; only the reporting changes. Record the skipped branches and write them to the job summary along with the sweep counters, so they stay visible without being buried in a 1,600-line log and without holding the run red. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: daca0885-6779-4adb-bb02-ff6920d73ab3
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
SHA mismatches are treated as proof of branch reuse without verifying an open pull request exists.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
.github/workflows/delete_staging_and_head_branches_writer.yaml — A SHA mismatch does not prove that a newer pull request is using the branch: it also occurs if… |
What changed in this PR
Updates branch cleanup to treat reused head branches as skips and report sweep results in the job summary.
Changes:
- Adds skip tracking for SHA mismatches.
- Adds reconciliation metrics and an exit-time summary.
| File | Description |
|---|---|
.github/workflows/delete_staging_and_head_branches_writer.yaml |
Handles branch skips and generates cleanup summaries. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A SHA mismatch was being reported as a self-healing skip on the assumption that a newer pull request had reused the branch. That assumption does not hold: a push after closure, or a ref deleted and recreated, produces the same mismatch. Those branches would have been reported as "no action needed" on every run while nothing ever collected them, so they would leak permanently. Confirm over the API that an open pull request currently has the branch as its head ref before skipping, and name that pull request in the warning and the job summary. An unexplained mismatch is now a failure that asks for investigation, and a failed lookup fails closed rather than being read as "nothing is using this branch". Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: daca0885-6779-4adb-bb02-ff6920d73ab3
aparnajyothi-y
approved these changes
Sep 3, 2026
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Skip-record write failures can be masked, producing a successful run with an incomplete summary.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
.github/workflows/delete_staging_and_head_branches_writer.yaml — This write is unguarded even though process_pr runs under if ! during a sweep, which disables… |
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
|
.github/workflows/delete_staging_and_head_branches_writer.yaml — A SHA mismatch does not prove that a newer pull request is using the branch: it also occurs if… View resolved comment |
Suppressed comments (1)
.github/workflows/delete_staging_and_head_branches_writer.yaml:365
- As in the same-ref path, this unguarded append can fail silently because the sweep invokes
process_prfrom anif !condition and the next command returns success. Explicitly propagate the recording failure so the run cannot claim success without reporting the skipped branch.
record_skip "${pr_number}" "${head_ref}" "${BLOCKING_PR}"
| delete_branch "${base_ref}" "${head_sha}" || return 1 | ||
| delete_branch "${base_ref}" "${head_sha}" || delete_status=$? | ||
| if (( delete_status == 3 )); then | ||
| record_skip "${pr_number}" "${base_ref}" "${BLOCKING_PR}" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #9332. That PR fixed the sweep; this one fixes the single condition it exposed.
What #9332 achieved
The first genuine post-merge run (
33774976735, running the merge commit) completed a full sweep for the first time since 19 Aug:The next run confirms steady state: 1,467 branches remaining, 1 target, 60 seconds. The backlog drained in one sweep.
The condition it exposed
Both runs still exit 1, for one reason:
This is
delete_branch's SHA guard, and it is working correctly. Traced:jdeniau-GHSA-xvcm-6775-5m9r@9ff0d0ef12cc7d, parent9ff0d0ef12cc7d— and it is still openDeleting that branch would have broken a live pull request. The guard prevented it.
Why this still needs changing
The behaviour is right; the severity is wrong.
The state is permanent for as long as PR 9226 stays open (last touched 26 Aug), so the workflow would be red on every run indefinitely — the same alert fatigue #9332 set out to remove, just from a different cause. We'd have traded a 404 for a stuck branch.
It's also miscategorised. Every other "nothing safe to do here" state already returns 0: pull request still open, fork-headed, deleted. A reused head branch belongs in that bucket.
And it needs no human intervention — it self-heals. Once PR 9226 closes, its own cleanup deletes the head branch; PR 9129's head then reads as
already absent→ returns 0 → the leftover staging branch is collected on the next sweep.The change
delete_branchreturns 3 for this case specifically, with a::warning::instead of::error::. A genuine deletion failure still returns 1, still fails the run, and still prevents the staging branch from being deleted after it.process_prtreats 3 as a recorded skip and returns 0.Nothing that was previously deleted is deleted any differently. Only the reporting changes.
Rendered summary:
Validation
actionlint,bash -n,shellcheckclean. 19/19 assertions pass, including:process_pr 9129against the live repo now returns 0, emits a warning not an error, records the skip, and leaves both branches in place (verified they still exist afterwards)GITHUB_STEP_SUMMARYis safe underset -uEXITtrap in placeNote for reviewers
jdeniau-GHSA-xvcm-6775-5m9rneeds no action — PR 9226 is legitimately using it. It resolves itself when that PR closes.Unchanged from #9332: ~1,300 of the remaining 1,467 branches are fork-headed and still out of scope, and the 63 orphaned staging branches still need a separate one-off decision.